[Feat] Convert GeoJson Features to GeoArrow (Draft) #2889

lixun910 · 2024-02-20T21:52:49Z

This WIP PR is trying to add functions to convert GeoJson Features to GeoArrow. These functions could be used as:

support write GeoArrow file format
support arrow-table loader's option in other loaders e.g. GeoJson/Csv to load spatial data into Arrow memory
support creating GeoArrow table in memory

I am not confident with the implementation in this PR (missing documentation in js apache-arrow). Please help to take a look if you are interested. Thank you!

kylebarron

Do you plan to handle converting properties to arrow as well? That requires handling at least a few different property types.

kylebarron · 2024-02-21T16:06:15Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  Vector as ArrowVector,
+  makeBuilder
+} from 'apache-arrow';
+import {


Maybe clearer to use import type if (I think) these are only used as types

kylebarron · 2024-02-21T16:12:21Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  let geometryType = '';
+  for (let i = 0; i < features.length; i++) {
+    const {geometry} = features[i];
+    if (i === 0) {
+      geometryType = geometry.type;
+    }
+    if (geometry.type.includes('Multi')) {
+      geometryType = geometry.type;
+      break;
+    }
+  }


Instead I'd suggest building a Set of geometry types instead, because you probably want to error if you see both Point and LineString geometries in a single feature collection (at least until you want to support mixed-geometry arrays, which have a decent amount of complexity).

kylebarron · 2024-02-21T16:18:39Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  // make geoarrow builder
+  const builder = makeBuilder({
+    type: pointType,
+    nullValues: [null]
+  });


Fwiw I've never tried the Arrow JS builders.

Like how the geojson-binary geojson conversion is implemented in loaders, in my own work I try to do a two-pass approach where I first count the number of coordinates and thus infer the sizes of buffers needed, and then in a second step allocate the exact size of buffers once and fill them with coordinates.

In your case if you wanted to go that route you could count the number of coordinates you'll have, instantiate a single Float64Array and set the values with coordinates. For nested types you'd have to manage building your own offset buffers as well, which may be a bit more complex than you want.

While your two-pass approach is surely more optimized It would be nice to build a baseline with the builders, as this presumably lets us work with more sanely with complex data types, that can have many index columns.

We can add such optimizations later.

kylebarron · 2024-02-21T16:20:17Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  // build geometry vector
+  const geometry = builder.finish().toVector();


If you ever want to handle attributes in the future, ideally each column will have identical chunking. Whereas when you let Arrow JS handle the builders, you don't necessarily have control over the chunking of each column.

Good point. I was assuming that we if we build the columns in parallel, we can choose to stop "building" after a certain number of rows and then create a RecordBatch from the "partial" vectors for each column, and then start building new colums/vectors again, rinse and repeat?

kylebarron · 2024-02-21T16:21:28Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+export function geojsonLineStringToArrow(name: string, lines: Feature[]): GeoArrowReturnType {
+  // get dimension from the first line
+  const dimension = (lines[0].geometry as LineString).coordinates[0].length;
+  const pointFieldName = dimension === 2 ? 'xy' : 'xyz';
+
+  // get line type
+  const nullable = false;
+  const coordType = new ArrowFloat();
+  const pointType = new FixedSizeList(
+    dimension,
+    new ArrowField(pointFieldName, coordType, nullable)
+  );
+  const lineType = new ArrowList(new ArrowField('points', pointType, nullable));
+
+  // create a field
+  const metaData: Map<string, string> = new Map([['ARROW:extension:name', 'geoarrow.linestring']]);
+  const field = new ArrowField(name, lineType, nullable, metaData);
+
+  // make geoarrow builder
+  const builder = makeBuilder({
+    type: lineType,
+    nullValues: [null]
+  });
+
+  for (let i = 0; i < lines.length; i++) {
+    const coords = (lines[i].geometry as LineString).coordinates;
+    // @ts-ignore
+    builder.append(coords);
+  }
+
+  const geometry = builder.finish().toVector();
+
+  return {
+    field,
+    geometry
+  };
+}


It seems like each of these 6 functions are exactly the same except for the creation of the types. You might be able to deduplicate the second half (filling) of each of these functions.

lixun910 · 2024-02-21T16:59:30Z

Thank you very much, @kylebarron! Great comments. I will address them. Yes, this PR is an initial work of converting other format e.g. GeoJSON/Csv to arrow, so I plan to handle attributes in the future. Do you know if there is already pure JS library doing this?

Another question: when you mention: in my own work I try to do a two-pass approach where I first count the number of coordinates and thus infer the sizes of buffers needed, could you point to me the code? The implementation in this PR is borrowed from OGR C++ code, which is based on the Apache arrow's C lib. I can't find the documentation of how to use makeBuilder() or how to create a nested list array with offset buffers...

Thanks again!

kylebarron · 2024-02-21T18:23:49Z

https://github.com/geoarrow/geoarrow-js/blob/main/src/algorithm/coords.ts has an example of creating arrays from raw Float64Arrays and Int32Arrays for offsets (though offsets there are not modified). You can look at the overloads on makeData for what input each type expects.

Signed-off-by: Xun Li <[email protected]>

ibgreen

This looks like a great start. I'd love to use it for adding arrow output from GeoJSONLoader and CSVLoader etc. For that we also need to be able to create at least a subset of normal / property columns (numbers, booleans, strings, maybe dates).

ibgreen · 2024-02-23T19:28:16Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

@@ -0,0 +1,375 @@
+import {
+  Float64 as ArrowFloat,


Since this renaming tends to get excessive, in other files I have been breaking my own rule (not to use wildcard imports) and used import * as arrow and then referenced as arrow.Float. I think this makes the code quite clear.

ibgreen · 2024-02-23T19:29:08Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+/**
+ * Arrow Field and geometry vector
+ */
+export type GeoArrowReturnType = {


Seems more general, overall I would like us to have support for all column types not just GeoArrow geometry:

Suggested change

export type GeoArrowReturnType = {

export type ArrowColumn = {

ibgreen · 2024-02-23T19:30:50Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+      break;
+    }
+  }
+  if (geometryType === 'Point') {


Nit: I find switch statments much easier to read.

ibgreen · 2024-02-23T19:31:11Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+    if (i === 0) {
+      geometryType = geometry.type;
+    }
+    if (geometry.type.includes('Multi')) {


Maybe more readable?

Suggested change

if (geometry.type.includes('Multi')) {

if (geometry.type.startsWith('Multi')) {

ibgreen · 2024-02-23T19:32:52Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  name: string,
+  points: Array<Feature | GeoJsonObject>
+): GeoArrowReturnType {
+  const firstPoint = isFeature(points[0]) ? points[0].geometry : points[0];


What does this do? Handle lists of just geometries? Do we need such dynamic support? It makes the code a little harder to follow.

ibgreen · 2024-02-23T19:42:15Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  // make geoarrow builder
+  const builder = makeBuilder({
+    type: pointType,
+    nullValues: [null]
+  });


While your two-pass approach is surely more optimized It would be nice to build a baseline with the builders, as this presumably lets us work with more sanely with complex data types, that can have many index columns.

We can add such optimizations later.

ibgreen · 2024-02-23T19:46:08Z

modules/arrow/src/geoarrow/convert-geojson-feature-to-arrow.ts

+  // build geometry vector
+  const geometry = builder.finish().toVector();


Good point. I was assuming that we if we build the columns in parallel, we can choose to stop "building" after a certain number of rows and then create a RecordBatch from the "partial" vectors for each column, and then start building new colums/vectors again, rinse and repeat?

lixun910 added 2 commits February 20, 2024 11:51

fix getBinaryPolygons for in-memory arrow table

3f06878

wip

793a3cd

lixun910 marked this pull request as draft February 20, 2024 21:52

lixun910 requested a review from kylebarron February 20, 2024 22:02

kylebarron reviewed Feb 21, 2024

View reviewed changes

wip

9aadea6

Signed-off-by: Xun Li <[email protected]>

ibgreen approved these changes Feb 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat] Convert GeoJson Features to GeoArrow (Draft) #2889

[Feat] Convert GeoJson Features to GeoArrow (Draft) #2889

lixun910 commented Feb 20, 2024 •

edited

Loading

kylebarron left a comment

kylebarron Feb 21, 2024

kylebarron Feb 21, 2024

kylebarron Feb 21, 2024

ibgreen Feb 23, 2024

kylebarron Feb 21, 2024

ibgreen Feb 23, 2024

kylebarron Feb 21, 2024

lixun910 commented Feb 21, 2024

kylebarron commented Feb 21, 2024

ibgreen left a comment

ibgreen Feb 23, 2024

ibgreen Feb 23, 2024

ibgreen Feb 23, 2024

ibgreen Feb 23, 2024

ibgreen Feb 23, 2024

ibgreen Feb 23, 2024

ibgreen Feb 23, 2024

		// build geometry vector
		const geometry = builder.finish().toVector();

	export type GeoArrowReturnType = {
	export type ArrowColumn = {

	if (geometry.type.includes('Multi')) {
	if (geometry.type.startsWith('Multi')) {

[Feat] Convert GeoJson Features to GeoArrow (Draft) #2889

Are you sure you want to change the base?

[Feat] Convert GeoJson Features to GeoArrow (Draft) #2889

Conversation

lixun910 commented Feb 20, 2024 • edited Loading

kylebarron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lixun910 commented Feb 21, 2024

kylebarron commented Feb 21, 2024

ibgreen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lixun910 commented Feb 20, 2024 •

edited

Loading